Restructure Net.Download and Net.DownloadText #2904
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DasSkelett
added
Core (ckan.dll)
HebaruSan
reviewed
Nov 3, 2019
This Kraken should be used in those cases, where we use cURL as fallback when native downloads fail. It has some nice features: The Kraken holds the response (header + body), the uRL, the previous network exceptions and the returned status code.
Restructure Net.Download and Net.DownloadText to allow better logging and catching of download errors. Throw `NAtiveAndCurlDownloadFailedKraken`s, as well as log more of what's happening.
DasSkelett
force-pushed
the
fix/throw-curl-errors
branch
from
a0c0a96
to
e9b9811
Compare
|
Okay, so now it always logs the answer if the download fails, and only with debug flag if the download succeeds. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
#2879 tried to get better insights why calls to GitHub by the bot were failing.
It was a partial success, we got the returned status code, but it also broke the custom error handling for SpaceDock for example.
Changes
First, I enabled the possibility to pass a header writeback function to CurlSharp. This allows catching the header of the response for logging purposes.
Passing it is optional.
Secondly, I created a new Kraken,
NativeAndCurlDownloadFailedKraken. This one is used in both above mentioned download methods. It has some nice additions to a plain Kraken/Exception (like accessing the response (header and/or content) or the status code) and is more usable thanWebExceptions.Thirdly, I added some logging to the two download methods. If the debug-flag is set, the entire header and the content (in case of
DownloadText) is written to the output. Plus, it is save in theNativeAndCurlDownloadFailedKraken, so accessing and deciding based on them higher up in the stack is easier.Also , parsing the returned json containing a error message from SpaceDock/CurseForge now works again. This fixes a failing unit test.
Furthermore in the two last commits, I introduced two custom error messages for GitHub if the rate limit is exceeded (based on return code 403 +
X-RateLimit-Remainingheader),and if CurseForge is blocking CKAN (based on return code 403).